fix(trace): the tape is the process's, not a connection worker's (#739, #755, #760) - #761
Merged
InauguralPhysicist merged 1 commit intoJul 29, 2026
Conversation
#755, #760) trace_shutdown() is a PROCESS-wide teardown — it closes the one tape, drops an embedder's trace sink, and shuts down the replay reader — and ext_http.c's per-connection worker called it when it finished a request. Each worker owns its own EigsState, but the tape belongs to the process, so the FIRST request served closed it and every later request's records were silently dropped. Measured on the unfixed runtime, 400 requests against a code route that assigns one marker each: records on tape: 1 (fd already closed) and after the fix, mid-run, same test: records on tape: 336 (fd still open) The same teardown freed the temporal prev-table, running slot_decref on entries recorded by other, still-live threads. The teardown is now split. trace_shutdown() keeps the process-wide half. A new trace_thread_release() releases only the calling thread's history, and eigs_thread_detach calls it for every thread — so the HTTP worker no longer touches the trace subsystem at all. Ordering is preserved: the release runs while eigs_current still points at the thread and before the freelists and arena go, and trace_shutdown still releases the process owner's own table before the global env dies (eigs_close). The prev-table moved onto EigsThread behind bridge macros. That is not a new scope — it is the scope the table always effectively had: it is keyed by INTERNED NAME POINTER and the intern table is already per-thread, so two threads' "x" were never the same key. Process-global storage bought nothing and cost ownership, and per-thread needs no lock because only the owning thread touches its table. Corrects one premise in the issue: cross-state history reads were NOT possible. Pointer-keying already separated them. The damage was the teardown, not the lookup. The tape ENCODING is untouched, so no format-version bump (#411) — this was an ownership bug, not a format one. Also in the same file: #755 — http_post silently sent NO headers when `headers` was a JSON object. The code tested VAL_LIST while eigs_json_parse_object returns a VAL_DICT, so the loop never ran and the request went out bare: no error, a normal response body, an Authorization header silently not sent. Both shapes are accepted now, and a headers argument that parses to neither raises type_mismatch instead of dropping them. BUILTINS.md never said which shape was expected; it does now. #760 — HS27/HS28 failed one CI run reporting "aggregate body budget (got 000)", a DoS-gate regression that had not happened: both checks had received EMPTY responses because the server was never reachable. Three measured defects — the listen port was drawn from 50000-59999, entirely inside the kernel's ephemeral range (32768-60999), so the suite's own curl clients could hold the port the next server tried to bind; the readiness loop's real budget was 3.58s, not the ~33s its --max-time implied, because a refused connection returns instantly; and neither failure was detected, so the test ran against a server that was not there while the log naming the cause was written to /tmp and never read. Shared pick_port + wait_ready helpers now, a setup failure reports itself as one and quotes the log, and the main server in the same file had both defects too. Gates: release 3273/3273; asan+ubsan 3277/3277 and asan-http 3373/3373, leak tally 0 in both; observer corpus 14/14; test_replay.sh 24/24; HTTP suite 42/42; freestanding symbol gate stage 1+2 OK; jit-smoke; embed stack soak OK. HS33 validated red against the unfixed runtime (got 1 request marker, expected >= 3). Closes #739 Closes #755 Closes #760 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes trace ownership boundaries and an HTTP client usability bug, and hardens the HTTP integration test harness so setup failures are diagnosed as such rather than misreported as feature regressions. It aligns the runtime with the multi-state/per-thread embedding seam by separating process-wide trace teardown (tape/sink/replay) from per-thread temporal history cleanup.
Changes:
- Split trace teardown into process-wide
trace_shutdown()and per-threadtrace_thread_release(), moving the temporal prev-table ontoEigsThreadand removing the erroneous per-request shutdown from HTTP workers. - Fix
http_postso headers provided as a JSON object are actually sent (and wrong parsed shapes raise instead of silently dropping headers), and document accepted header shapes. - Add shared port-picking/readiness helpers and new regression gates (HS33/HS34) to
tests/test_http_server.shto prevent flaky setup and validate the fixes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_http_server.sh | Adds port/readiness helpers and HS33/HS34 regression tests for trace-tape survival and http_post header shapes; improves setup-failure diagnosis. |
| src/trace.h | Documents that trace_shutdown() is process-wide and introduces trace_thread_release() for per-thread history cleanup. |
| src/trace.c | Moves the temporal prev-table to EigsThread via bridge macros and implements trace_thread_release(); keeps tape/sink/replay process-wide. |
| src/state.c | Calls trace_thread_release() during thread detach so workers clean up their own history without touching process trace state. |
| src/ext_http.c | Accepts JSON object headers in http_post, errors on wrong parsed shapes, and removes trace_shutdown() from the connection worker. |
| src/eigenscript.h | Adds prev_tab/prev_cap/prev_count to EigsThread and bridge macros for trace’s per-thread prev-table. |
| docs/TRACE.md | Documents the per-thread history vs per-process tape ownership rule and the worker prohibition on trace_shutdown(). |
| docs/EMBEDDING.md | Clarifies that trace sink/tape are process-scoped across multiple states and points to trace_thread_release() for worker cleanup. |
| docs/BUILTINS.md | Documents http_post header JSON shapes (object or flat array), error behavior, and limits. |
| CHANGELOG.md | Records fixes for #739/#755/#760 and the associated test harness change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| curl -s --max-time 5 "http://127.0.0.1:$PORT3/hit" > /dev/null 2>&1 | ||
| done | ||
| sleep 0.5 | ||
| MARKERS=$(grep -c '^A zz=' "$TAPE3" 2>/dev/null || echo 0) |
InauguralPhysicist
deleted the
fix/739-trace-tape-per-connection-teardown
branch
July 29, 2026 03:40
This was referenced Jul 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#739 — every HTTP connection worker tore down the process tape
trace_shutdown()is a process-wide teardown: it closes the one tape, drops an embedder's trace sink, and shuts down the replay reader.ext_http.c's per-connection worker called it when it finished a request. Each worker owns its ownEigsState, but the tape belongs to the process — so the first request served closed it, and every later request's records were silently dropped.Measured, 400 requests against a code route that stamps one marker each:
The same teardown also freed the temporal prev-table, running
slot_decrefon entries recorded by other, still-live threads.Fix. The teardown splits.
trace_shutdown()keeps the process-wide half; a newtrace_thread_release()releases only the calling thread's history, andeigs_thread_detachcalls it for every thread — so the HTTP worker no longer touches the trace subsystem at all. Ordering is preserved: the release runs whileeigs_currentstill points at the thread and before the freelists and arena go, andtrace_shutdownstill releases the process owner's own table before the global env dies (the constrainteigs_closedocuments).The prev-table moved onto
EigsThreadbehind bridge macros. That is not a new scope — it is the scope the table always effectively had: it is keyed by interned name pointer, and the intern table (g_env_name_interns) is already per-thread, so two threads'xwere never the same key. Process-global storage bought nothing and cost ownership. It needs no lock, because only the owning thread touches its table.One premise in the issue is wrong, and worth recording: it says
prev of xin one state can read another's history. It cannot — pointer-keying already separated them. The damage was the teardown, not the lookup.No format-version bump. The tape encoding is untouched (#411 applies to encoding changes); this was an ownership bug.
#755 —
http_postsilently sent no headers for the natural shapeThe signature is
http_post of [url, headers, body], and a JSON object is what a caller reaches for first. The code testedVAL_LISTwhileeigs_json_parse_objectreturns aVAL_DICT, so the loop never ran and the request went out bare — no error, no warning, a normal-looking response body. AnAuthorizationheader silently not sent is the worst version of that.docs/BUILTINS.mdnever said which shape was expected.Both shapes are accepted now; a headers argument that parses to neither raises
type_mismatchrather than dropping them (an unparseable argument, including"", still means "no headers" — the documented idiom). BUILTINS.md now states the accepted shapes.#760 — the gate that misdiagnosed itself
HS27/HS28failed one CI run with "aggregate body budget (got 000)" — a DoS-gate regression that had not happened. Both checks had received empty responses because the server was never reachable. Three defects, each measured:50000-59999, entirely inside the kernel's ephemeral range (32768-60999on Linux), so this suite's owncurlclients could hold the port the next server tried to bind;--max-time 1implied — a refused connection returns instantly, so the loop was effectively30 × sleep 0.1;/tmpand was never read.Shared
pick_port(below the ephemeral floor, read from/procwhere available) andwait_ready(wall-clock deadline) helpers now; a server that never comes up fails as a setup failure quoting its own log. The main server in the same file had both defects and now uses the helpers.Validation
New gate
HS33observes the tape by content — no/proc(macOS runs this suite) and no assumption about stdio buffer size: each request stamps a distinct marker from the shared store first, then writes padding that pushes it out of the buffer, so a marker on disk means that request's records really reached the tape. Validated red against the unfixed runtime (got 1 request markers (expected >= 3)), green with the fix.HS34covers #755 in all three directions: object shape delivers, array shape still delivers, wrong shape raises.test_replay.sh24/24 (it drivesEIGS_REPLAY_STRICT=1per case internally)docs/TRACE.mdgains the ownership rule (process tape vs per-thread history, and that a worker must never calltrace_shutdown);docs/EMBEDDING.mdgains the multi-state consequence for hosts installing a trace sink.Closes #739
Closes #755
Closes #760
🤖 Generated with Claude Code